RemoveUnusedLabels: preserve comments and Kotlin-referenced labels - #966

Suggested review order: 16 of 52 (Score: 7)
Review first: openrewrite/rewrite-testing-frameworks#1093

What's changed?

Two things change, one for Kotlin and one for comments.

First, RemoveUnusedLabels now counts a Kotlin labeled return, return@name parsed as K.Return, and a Kotlin qualified this, this@name parsed as K.This, as references to the label name. On main the reference search looks only at J.Break and J.Continue, so it sees neither form and removes a label that is in use.

Second, the recipe now keeps the comments belonging to a label it removes. On main the visitor returns l.getStatement().withPrefix(l.getPrefix()), overwriting the labeled statement's prefix instead of merging the two, so a comment sitting in that prefix is dropped.

The new prefix keeps the whitespace of the label's prefix, so indentation does not change, and its comments are the concatenation, in source order, of the label's prefix, the space between the label name and the colon (a Space of its own in the LST that can hold comments, as in label /* an odd place */ : while (true) { ... }), and the statement's own prefix, which holds anything written after the colon.

The Kotlin check runs only when rewrite-kotlin is on the runtime classpath, which is not guaranteed, since it is declared as a provided dependency. It is guarded by a ReflectionUtils.isClassAvailable("org.openrewrite.kotlin.tree.K") constant, the same mechanism on the same class name that this repository's KotlinFileChecker uses.

Because the recipe now recognises two more kinds of reference, its description changes to "Remove labels that are not referenced by any break or continue statement or by a Kotlin labeled return or this expression", and the RemoveUnusedLabels row of the checked-in generated file src/main/resources/META-INF/rewrite/recipes.csv carries the same text.

What's your motivation?

Recipe: org.openrewrite.staticanalysis.RemoveUnusedLabels.

Case 1: Kotlin labeled return

Before

items.forEach callback@{ if (stop) return@callback }

Actual after the recipe

items.forEach { if (stop) return@callback }

Expected after the recipe

(unchanged)

The generated Kotlin does not compile because return@callback refers to a label that the recipe deleted. RemoveUnusedLabels is part of org.openrewrite.staticanalysis.CodeCleanup, so applying code cleanup to Kotlin can break the build.

Case 2: comment attached to a Java label

Before

unused: /* why this loop exists */
while (condition()) { break; }

Actual after the recipe

while (condition()) { break; }

Expected after the recipe

/* why this loop exists */
while (condition()) { break; }

The Java output compiles, but it loses the comment attached to the label. Both defects reproduce on v2.39.0, v2.40.0, and current main, which carry the same implementation.

Confirmed real-world execution

• Project: Jellyfin Android (2,695 stars on 2026-08-16).
• Location: MainActivity.kt at 75a504b6.
• Recipe release: org.openrewrite.recipe:rewrite-static-analysis:2.41.0.

The released recipe removes the explicit callback@ label and leaves two return@callback statements. The generated Kotlin does not compile because those returns refer to a label that no longer exists.

Anything in particular you'd like reviewers to focus on?

No existing test expectation changed. The six tests already in RemoveUnusedLabelsTest keep their input and their expected output.

Two limits, both describing the behaviour with this change applied:

• The Kotlin check matches by name, not by scope. If a return@x or a this@x inside the labeled statement uses the same name as an unused outer label, the recipe keeps that outer label. The check is conservative: it can keep a safe-to-remove label, but it does not remove a referenced label.
• A C# label reached only by a goto is still removed: Cs.GotoStatement holds its target as a plain Expression, not as a reference to a label, so the search cannot recognise the use. Main removes such a label too, so this change neither introduces that behaviour nor fixes it.

Have you considered any alternatives or workarounds?

One option is to skip Kotlin files altogether, with Preconditions.check(Preconditions.not(new KotlinFileChecker<>()), ...). This repository's own CLAUDE.md names that idiom as the way to exclude a language: use Preconditions.check() to exclude specific file types, Kotlin files among them, when a recipe is language specific. That would be a smaller change, roughly 40 lines against the 331 added here, but the recipe would then never remove any Kotlin label at all, including the unused ones it removes correctly today and still removes with this change. Tell me if you prefer it.

Any additional context

This change adds 11 tests to RemoveUnusedLabelsTest, taking it from 6 tests to 17. It deletes or renames no existing test. Without the code change in this pull request, 8 of the new tests fail. They cover these cases:

• Kotlin references through return@name and this@name
• comments in the label prefix and after the colon, merged in source order
• a comment between the label name and the colon
• comments on blocks, do/while loops, switch statements, and expression statements
• a block comment after the colon
• nested labels
• a nested Kotlin label that repeats an outer name

The test methods name each exact case in the changed test file.

The other 3 added tests are controls: two show that an unused Kotlin label is still removed, one on a while loop and one on a lambda, and one shows that a Java label in use keeps both the label and its comment.

This change was prepared with AI assistance (Claude Code). I reviewed the code, the tests and this description.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've run ./gradlew build locally, and committed any resulting changes to recipes.csv
• I've formatted the lines I changed, without reformatting code I didn't touch